Skip to content

Update agent config defaults and format#959

Merged
aphralG merged 5 commits intov3from
fix-config-structs-and-defaults
Jan 15, 2025
Merged

Update agent config defaults and format#959
aphralG merged 5 commits intov3from
fix-config-structs-and-defaults

Conversation

@aphralG
Copy link
Contributor

@aphralG aphralG commented Jan 9, 2025

Proposed changes

Fixed defaults and formatting for the client portion of agent config:

  • Renamed Common Settings struct to BackOff and moved into to the Client struct
  • Created HTTP struct for the Client HTTP config settings
  • GRPC config settings are now in the Client Struct
  • Set Defaults for KeepAlive and HTTP settings in Client - Made sure defaults were being used for client settings

Client section of the agent config below works and defaults are set to the values below.

client: 
  http:
    timeout: 10s                      # HTTP client timeouts
  grpc:
    keepalive:
      timeout: 10s                   # Client timeout
      time: 20s                      # Total time for client requests
      permit_without_stream: true    # Allow connections without streams
    max_message_size: 1048576        # Max size for client messages (in bytes)
    max_message_receive_size: 1048576 # Max size for receiving client messages
    max_message_send_size: 1048576   # Max size for sending client messages
  backoff:
    initial_interval: 500ms         # Initial backoff interval
    max_interval: 10s                # Maximum backoff interval
    max_elapsed_time: 30s           # Maximum time spent on retries
    randomization_factor: 0.5       # Factor for randomizing the backoff interval
    multiplier: 1.5                 # Backoff multiplier

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

@aphralG aphralG self-assigned this Jan 9, 2025
@aphralG aphralG requested a review from a team as a code owner January 9, 2025 11:55
@github-actions github-actions bot added the chore Pull requests for routine tasks label Jan 9, 2025
commonSettings := &config.BackOff{
InitialInterval: cs.agentConfig.Client.Backoff.InitialInterval,
MaxInterval: cs.agentConfig.Client.Backoff.MaxInterval,
MaxElapsedTime: cs.agentConfig.Client.Backoff.MaxElapsedTime,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the Subscribe & CreateConnection RPCs, we always want to retry indefinitely. Otherwise if we stop retrying then the agent will just keep running doing nothing and never connect to the management plane.

Hence why the MaxElapsedTime is set to 0 for these RPCs.

@aphralG aphralG requested a review from dhurley January 14, 2025 10:38
@aphralG aphralG merged commit b6e143c into v3 Jan 15, 2025
20 checks passed
@aphralG aphralG deleted the fix-config-structs-and-defaults branch January 15, 2025 11:48
sean-breen pushed a commit that referenced this pull request Jan 15, 2025
* update config defaults and format
sean-breen added a commit that referenced this pull request Jan 28, 2025
* support reading token from file via config

* remove empty file

* simplify token validation and add unit tests

* add unit tests for transport credentials funtions

* address PR feedback

* proto updates

* fix function name

* fix lint error: lll

* add missing PR feedback

* remove error log message

* fix unit test

* Fix apk test package naming (#961)

* modify alpine package name:
nginx-agent-3.0.0_1234 -> nginx-agent-3.0.0.1234

* protoc-gen update

* Update agent config defaults and format (#959)

* update config defaults and format

* Add config apply request queue (#949)

* add unit tests for transport credentials funtions

* fix test name

* fix error message

* fix lint error: lll

* modify error messages

* remove error logging and modify messages

* fall back to token field if error occurs when reading file

* fix bad merge

* restarting gRPC conn

* remove code from testing

* fix lint errors: whitespace, revive

* use tokenpath as config option, fixes problem with cli param parsing

* correct yaml key in AuthConfig

* proto updates

---------

Co-authored-by: aphralG <108004222+aphralG@users.noreply.github.com>
Co-authored-by: Donal Hurley <djhurley1990@gmail.com>
sean-breen added a commit that referenced this pull request Mar 3, 2025
* support reading token from file via config

* remove empty file

* simplify token validation and add unit tests

* add unit tests for transport credentials funtions

* address PR feedback

* proto updates

* fix function name

* fix lint error: lll

* add missing PR feedback

* remove error log message

* fix unit test

* Fix apk test package naming (#961)

* modify alpine package name:
nginx-agent-3.0.0_1234 -> nginx-agent-3.0.0.1234

* protoc-gen update

* Update agent config defaults and format (#959)

* update config defaults and format

* Add config apply request queue (#949)

* add unit tests for transport credentials funtions

* fix test name

* fix error message

* fix lint error: lll

* modify error messages

* remove error logging and modify messages

* fall back to token field if error occurs when reading file

* fix bad merge

* restarting gRPC conn

* remove code from testing

* fix lint errors: whitespace, revive

* add new topic for handling Token file updates

* add CredentialWatcherService

* adding initial watcher for credential files

* trigger connection reset after credential update

* added ConnectionResetTopic and event processing

* Automatically add token-path to Credential watcher

* add function to check credential paths defined in agent config

* fix lint

* use tokenpath as config option, fixes problem with cli param parsing

* add CredentialWatcherService + tests

* fix lint errors

* updates to generated files

* Send create connection after disconnect from management plane (#967)

* correct yaml key in AuthConfig

* fix: flaky test (#968)

`TestConvertToStructs` was occasionally failing because it was expecting `range` over a `map` to be a consistent order, but per [spec]:

> The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next.

Uses `ElementsMatch` so the test passes even when the order of elements is different.

[spec]: https://go.dev/ref/spec#RangeClause

* update tests

* update tests

* wait for create connection

* PR feedback

* fix race condition

* clean up

* clean up

* update grpc connection in command and file plugins

* move log message and fix file_plugin_test.go

* fix lint

* handle command and file service client updates after grpc reset

* update FakeCommandService

* add unit tests

* fix bad test

* formatting

* remove test

* increase timeout before checking connection after restart

* set isConnected to false when handling subscribe errors

* increase code coverage

* lint fix

* fieldalignment

* remove unused fake

* PR feedback

* update fake command service

* more PR feedback

* PR feedback: disable watcher during config apply

* don't pause credential watcher during config apply

* debug log messages

* Add mutex around updating client in the CommandService

* undo change to metric name

* update test

* lock watcher when replacing grpc connection

* add lock around subscibe client access

* fix lint error

* unlock watcher mutex in error case

* return with no blank line

* lint error ?

* handle rename cases

* remove unneccessary cases from switch

* rewrite switch as if

* handling kubernetes secret update case

* fix test

* save connection status when resetting file manager service

---------

Co-authored-by: aphralG <108004222+aphralG@users.noreply.github.com>
Co-authored-by: Donal Hurley <djhurley1990@gmail.com>
Co-authored-by: Ryan Davis <ry.davis@f5.com>
Co-authored-by: Aphral Griffin <a.griffin@f5.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Pull requests for routine tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants